[fix](ci) Restrict trigger push branch for GitHub Workflow#65121
[fix](ci) Restrict trigger push branch for GitHub Workflow#65121apupier wants to merge 1 commit into
Conversation
Feature branches rarely need their own CI runs: the code is already tested when a pull request is opened against a release branch. If the push trigger has no branch restriction and pull_request is also configured, every push to a branch with an open PR runs the workflow twice: once for the push and once for the PR synchronisation. Always give the push trigger an explicit list of branches: this stops branches created from a release branch from inheriting its workflow runs. see https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=430408443#GitHubActionsRecommendedPractices-Restrictthepushtriggertospecificbranches Signed-off-by: Aurélien Pupier <apupier@ibm.com>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
apupier
changed the title
apupier
changed the title
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the two workflow trigger changes. I found one blocking issue: branch-* still includes branch-named non-release/development heads, so duplicate push and pull_request_target runs remain possible in both workflows.
Critical checkpoint conclusions:
- Goal: not fully met because the broad branch glob still leaves duplicate runs for branch-named upstream heads.
- Scope: small and focused.
- Concurrency, lifecycle, config, persistence, protocol, and data writes: not applicable; this is a workflow trigger-only change.
- Parallel paths: both changed workflows have the same issue and both are commented inline.
- Tests/validation: static review plus Ruby YAML parse of both workflow files;
actionlintis unavailable in this runner. - User focus: no additional user-provided focus.
Subagent conclusions:
- optimizer-rewrite: no candidates; convergence round C-1 returned
NO_NEW_VALUABLE_FINDINGS. - tests-session-config:
TSC-1was accepted asM-1and submitted as inline commentsPF-1/PF-2; convergence round C-1 returnedNO_NEW_VALUABLE_FINDINGS.
| push: | ||
| branches: | ||
| - master | ||
| - branch-* |
There was a problem hiding this comment.
branch-* is still broad enough to include ordinary upstream development heads, so it leaves the duplicate push+PR workflow runs this PR is trying to avoid. Live branch metadata currently has unprotected matches such as branch-catalog-spi, branch-fs-spi, branch-hudi-mtmv, branch-refactor_property, and branch-4.1-fix-spill; pushing to any of those with an open PR would still trigger both the push run here and the pull_request_target run. Please restrict this to the actual long-lived release branches that should keep push CI, or add explicit exclusions for branch-named feature heads.
| push: | ||
| branches: | ||
| - master | ||
| - branch-* |
There was a problem hiding this comment.
Same issue as in code-checks.yml: branch-* still matches branch-named upstream development heads such as branch-catalog-spi, branch-fs-spi, branch-hudi-mtmv, branch-refactor_property, and branch-4.1-fix-spill. Those pushes would continue to run this workflow in addition to the pull_request_target run for an open PR, so the push filter should be narrowed to the intended long-lived release branches or explicitly exclude branch-named feature heads.
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
|
PR approved by anyone and no changes requested. |
|
run buildall |
3 participants
Feature branches rarely need their own CI runs: the code is already tested when a pull request is opened against a release branch. If the push trigger has no branch restriction and pull_request is also configured, every push to a branch with an open PR runs the workflow twice: once for the push and once for the PR synchronisation.
Always give the push trigger an explicit list of branches: this stops branches created from a release branch from inheriting its workflow runs.
see https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=430408443#GitHubActionsRecommendedPractices-Restrictthepushtriggertospecificbranches
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)